Refactor install-age calculation into AppInstall.getInstallAge()#9004
Refactor install-age calculation into AppInstall.getInstallAge()#9004lmac012 wants to merge 1 commit into
Conversation
Extract the elapsed-time-since-install logic out of DuckChatJSHelper behind a new suspend AppInstall.getInstallAge(): Duration?. Pixel bucketing semantics are unchanged. Asana: https://app.asana.com/1/137249556945/project/488551667048375/task/1215957241503520?focus=true Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
malmstein
left a comment
malmstein
left a comment
There was a problem hiding this comment.
Reviewed statically — pure behaviour-preserving refactor with green CI. Nice cleanup moving the install-age math into AppInstall.getInstallAge(): Duration?, which encapsulates the io() dispatch and the null edge cases behind the api. Bucket boundaries preserved exactly, new AppInstallRepositoryTest covers the branches, and dispatcherProvider is still used elsewhere so no dead dep. One non-blocking nit inline.
| @Test | ||
| fun whenInstallTimestampNotRecordedThenInstallAgeIsOmitted() = runTest { | ||
| whenever(mockAppInstall.getInstallationTimestamp()).thenReturn(0L) | ||
| fun whenInstallAgeIsNullBecauseNotRecordedThenInstallAgeIsOmitted() = runTest { |
There was a problem hiding this comment.
nit: now that the future/not-recorded distinction lives in getInstallAge(), this test and whenInstallAgeIsNullBecauseInFutureThenInstallAgeIsOmitted above have identical bodies (getInstallAge() → null → omitted). the two scenarios are already covered properly in AppInstallRepositoryTest, so we could collapse these into one "null age → param omitted" test here.
2 participants
Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1215957241503520?focus=true
Tech Design URL (if applicable):
Description
Extracts the "time since install" calculation out of
DuckChatJSHelperbehind a newsuspend fun AppInstall.getInstallAge(): Duration?(kotlin.time).getInstallAgeBucket()is now a thin mapping from the returnedDurationto the existing buckets, and thewithContext(io)+ clock +Instant/ChronoUnitplumbing moves intoAppInstallRepository.getInstallationTimestamp()is unchanged, so the attributed-metrics consumers are untouched. The Duck.ai install-age pixel semantics are unchanged (null when the timestamp is not recorded or is in the future).Note: this adds a method to the
:browser-apiAppInstallpublic API; a separate API proposal accompanies it.Steps to test this PR
QA-optional
No UI changes